feat(task-input): pin recently-used repos atop the cloud repo picker#2630
Draft
andrewm4894 wants to merge 1 commit into
Draft
feat(task-input): pin recently-used repos atop the cloud repo picker#2630andrewm4894 wants to merge 1 commit into
andrewm4894 wants to merge 1 commit into
Conversation
Selecting a cloud repository meant scrolling or typing past a long GitHub repo list every time, even though most users reuse the same few repos. Track the most recently selected cloud repositories (MRU, capped at 5) in settings and surface the top 3 still-connected ones in a "Recent" section pinned above the full list in the new-task repo picker. The pinned section only shows while idle — once the user starts searching, normal results take over. Generated-By: PostHog Code Task-Id: 29622575-d4f9-43c6-acd7-d20cc8ee2e98
|
React Doctor could not complete this scan.
Reviewed by React Doctor for commit |
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/ui/src/features/task-detail/components/TaskInput.tsx:219-221
The `connected` set is built from `repositories` (whose strings come from the API via `Object.keys(repositoryMap)` without normalization), but `recentCloudRepositories` entries are always stored lowercase by `addRecentCloudRepository`. A case-sensitive `Set.has()` lookup will miss any repo whose API name contains uppercase letters (e.g., `"PostHog/posthog"`), silently keeping the Recent section empty even after the user has picked that repo. The existing `normalizeRepoKey` helper in `repositories.ts` exists precisely because API-returned names are not guaranteed lowercase.
```suggestion
const connected = new Set(repositories.map((r) => r.toLowerCase()));
return recentCloudRepositories
.filter((repo) => connected.has(repo))
```
### Issue 2 of 3
packages/ui/src/features/folder-picker/GitHubRepoPicker.tsx:114-117
The `pinnedRecentSet` contains lowercased strings (from the store), but `repositories` items may carry their original API casing. The case-sensitive `Set.has` check means a mixed-case repo like `"PostHog/code"` passes the filter and ends up in `displayRepositories` a second time — once as the pinned lowercase entry and once from the unfiltered tail of `repositories`. Normalising the comparison here closes the gap.
```suggestion
return [
...pinnedRecentRepositories,
...repositories.filter((repo) => !pinnedRecentSet.has(repo.toLowerCase())),
];
```
### Issue 3 of 3
packages/ui/src/features/folder-picker/GitHubRepoPicker.tsx:265
Likewise, `isPinned` is computed with a case-sensitive lookup against `pinnedRecentSet`. If a mixed-case repo reaches this render path, the clock icon and divider will not render correctly. Normalising here keeps the rendering consistent with the deduplication logic above.
```suggestion
const isPinned = pinnedRecentSet.has(repo.toLowerCase());
```
Reviews (1): Last reviewed commit: "feat(task-input): pin recently-used repo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+219
to
+221
| const connected = new Set(repositories); | ||
| return recentCloudRepositories | ||
| .filter((repo) => connected.has(repo)) |
Contributor
There was a problem hiding this comment.
connected set is built from repositories (whose strings come from the API via Object.keys(repositoryMap) without normalization), but recentCloudRepositories entries are always stored lowercase by addRecentCloudRepository. A case-sensitive Set.has() lookup will miss any repo whose API name contains uppercase letters (e.g., "PostHog/posthog"), silently keeping the Recent section empty even after the user has picked that repo. The existing normalizeRepoKey helper in repositories.ts exists precisely because API-returned names are not guaranteed lowercase.
Suggested change
| const connected = new Set(repositories); | |
| return recentCloudRepositories | |
| .filter((repo) => connected.has(repo)) | |
| const connected = new Set(repositories.map((r) => r.toLowerCase())); | |
| return recentCloudRepositories | |
| .filter((repo) => connected.has(repo)) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/task-detail/components/TaskInput.tsx
Line: 219-221
Comment:
The `connected` set is built from `repositories` (whose strings come from the API via `Object.keys(repositoryMap)` without normalization), but `recentCloudRepositories` entries are always stored lowercase by `addRecentCloudRepository`. A case-sensitive `Set.has()` lookup will miss any repo whose API name contains uppercase letters (e.g., `"PostHog/posthog"`), silently keeping the Recent section empty even after the user has picked that repo. The existing `normalizeRepoKey` helper in `repositories.ts` exists precisely because API-returned names are not guaranteed lowercase.
```suggestion
const connected = new Set(repositories.map((r) => r.toLowerCase()));
return recentCloudRepositories
.filter((repo) => connected.has(repo))
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+114
to
+117
| return [ | ||
| ...pinnedRecentRepositories, | ||
| ...repositories.filter((repo) => !pinnedRecentSet.has(repo)), | ||
| ]; |
Contributor
There was a problem hiding this comment.
pinnedRecentSet contains lowercased strings (from the store), but repositories items may carry their original API casing. The case-sensitive Set.has check means a mixed-case repo like "PostHog/code" passes the filter and ends up in displayRepositories a second time — once as the pinned lowercase entry and once from the unfiltered tail of repositories. Normalising the comparison here closes the gap.
Suggested change
| return [ | |
| ...pinnedRecentRepositories, | |
| ...repositories.filter((repo) => !pinnedRecentSet.has(repo)), | |
| ]; | |
| return [ | |
| ...pinnedRecentRepositories, | |
| ...repositories.filter((repo) => !pinnedRecentSet.has(repo.toLowerCase())), | |
| ]; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/folder-picker/GitHubRepoPicker.tsx
Line: 114-117
Comment:
The `pinnedRecentSet` contains lowercased strings (from the store), but `repositories` items may carry their original API casing. The case-sensitive `Set.has` check means a mixed-case repo like `"PostHog/code"` passes the filter and ends up in `displayRepositories` a second time — once as the pinned lowercase entry and once from the unfiltered tail of `repositories`. Normalising the comparison here closes the gap.
```suggestion
return [
...pinnedRecentRepositories,
...repositories.filter((repo) => !pinnedRecentSet.has(repo.toLowerCase())),
];
```
How can I resolve this? If you propose a fix, please make it concise.| </ComboboxItem> | ||
| )} | ||
| {(repo: string) => { | ||
| const isPinned = pinnedRecentSet.has(repo); |
Contributor
There was a problem hiding this comment.
isPinned is computed with a case-sensitive lookup against pinnedRecentSet. If a mixed-case repo reaches this render path, the clock icon and divider will not render correctly. Normalising here keeps the rendering consistent with the deduplication logic above.
Suggested change
| const isPinned = pinnedRecentSet.has(repo); | |
| const isPinned = pinnedRecentSet.has(repo.toLowerCase()); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/folder-picker/GitHubRepoPicker.tsx
Line: 265
Comment:
Likewise, `isPinned` is computed with a case-sensitive lookup against `pinnedRecentSet`. If a mixed-case repo reaches this render path, the clock icon and divider will not render correctly. Normalising here keeps the rendering consistent with the deduplication logic above.
```suggestion
const isPinned = pinnedRecentSet.has(repo.toLowerCase());
```
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Picking a cloud repository when creating a task means scrolling or typing past a long GitHub repo list every time — even though most people reuse the same handful of repos (e.g. always typing
posthog/code). Reported in Slack: it'd be much faster if the few most recently used repos sat above the full list.Changes
recentRepositoriesprop onGitHubRepoPickeris optional, so the settings/inbox usages are unchanged.How did you test this?
settingsStore.test.ts—pnpm --filter @posthog/ui test settingsStorepasses (12/12).pnpm --filter @posthog/ui typecheckandbiome linton the touched files pass.Automatic notifications
Created with PostHog Code from a Slack thread